Skip to content

[SYCL][P2P] Fix info query for P2P #15873

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 7 commits into from
Oct 31, 2024
Merged

[SYCL][P2P] Fix info query for P2P #15873

merged 7 commits into from
Oct 31, 2024

Conversation

@JackAKirk
Copy link
Contributor

@JackAKirk JackAKirk commented Oct 25, 2024 •
edited
Loading

There has been some confusion I think originating in the fact that L0 backend returned bool type instead of int for P2P queries. This issue is fixed here oneapi-src/unified-runtime#2246

This PR correspondingly updates the dpc++ runtime.

Fixes #15841

@JackAKirk JackAKirk requested review from a team as code owners October 25, 2024 13:48
@JackAKirk JackAKirk requested a review from bso-intel October 25, 2024 13:48
@JackAKirk JackAKirk mentioned this pull request Oct 25, 2024
Merged
@JackAKirk JackAKirk changed the title [sycl][P2P] Fix info query for P2P [SYCL][P2P] Fix info query for P2P Oct 25, 2024
@JackAKirk JackAKirk mentioned this pull request Oct 25, 2024
Closed
@JackAKirk
Copy link
Contributor Author

JackAKirk commented Oct 29, 2024 •
edited
Loading

@bso-intel Could you take a quick look at this so that we can merge these fixes?

Cmake will be updated when oneapi-src/unified-runtime#2246 is merged, so no need to worry about that.

Thanks

@pbalcer
Copy link
Contributor

pbalcer commented Oct 29, 2024

I merged the UR PR, please update the tags.

@JackAKirk
Copy link
Contributor Author

JackAKirk commented Oct 30, 2024

I merged the UR PR, please update the tags.

Thanks, done.

@JackAKirk JackAKirk mentioned this pull request Oct 30, 2024
Merged
@pbalcer
Copy link
Contributor

pbalcer commented Oct 30, 2024

@intel/llvm-reviewers-runtime please review, this is blocking UR updates.

uditagarwal97
uditagarwal97 approved these changes Oct 30, 2024
View reviewed changes
Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one minor suggestion.

pbalcer
pbalcer reviewed Oct 31, 2024
View reviewed changes
@JackAKirk
Copy link
Contributor Author

JackAKirk commented Oct 31, 2024

All reviewers comments have been resolved and the UR tag is already updated. @intel/llvm-gatekeepers this is ready to merge.
Thanks

@sommerlukas sommerlukas merged commit e08b06f into intel:sycl Oct 31, 2024
13 checks passed
@sarnex
Copy link
Contributor

sarnex commented Oct 31, 2024

@JackAKirk Could this HIP postcommit failure be related?

Failed Tests (1):
  SYCL :: Graph/Explicit/event_status_querying.cpp

https://github.com/intel/llvm/actions/runs/11612687147/job/32337271582

@JackAKirk
Copy link
Contributor Author

JackAKirk commented Oct 31, 2024

@JackAKirk Could this HIP postcommit failure be related?

Failed Tests (1):
  SYCL :: Graph/Explicit/event_status_querying.cpp

https://github.com/intel/llvm/actions/runs/11612687147/job/32337271582

From looking at that Graph test and its includes I don't see how this PR could be related to that failure.
If there was not a recent PR that affects that code then this looks likely to be another flaky failure as a result of having a non officially supported ROCM card in the CI:

We have raised this issue several times both internally to line-management (and above) as well as via public issues. Here are a few examples I quickly found:

#7634 (comment)
#12997 (comment)
#14404
#14947

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pbalcer pbalcer pbalcer approved these changes

@uditagarwal97 uditagarwal97 uditagarwal97 approved these changes

@bso-intel bso-intel Awaiting requested review from bso-intel bso-intel is a code owner automatically assigned from intel/llvm-reviewers-runtime

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

P2P Access Not Working on Level Zero

5 participants

@JackAKirk @pbalcer @sarnex @uditagarwal97 @sommerlukas